Skip to content

Add presubmit job to kubernetes-sigs/node-readiness-controller#37035

Open
vitorfloriano wants to merge 1 commit into
kubernetes:masterfrom
vitorfloriano:prow-node-readiness-controller-presub-test
Open

Add presubmit job to kubernetes-sigs/node-readiness-controller#37035
vitorfloriano wants to merge 1 commit into
kubernetes:masterfrom
vitorfloriano:prow-node-readiness-controller-presub-test

Conversation

@vitorfloriano
Copy link
Copy Markdown
Contributor

This PR adds a presubmit job for running tests to kubernetes-sigs/node-readiness-controller.

Follow-up to kubernetes-sigs/node-readiness-controller#210 (comment)

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vitorfloriano
Once this PR has been reviewed and has the lgtm label, please assign mrunalp for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/config Issues or PRs related to code in /config area/jobs labels May 14, 2026
@k8s-ci-robot k8s-ci-robot requested a review from ajaysundark May 14, 2026 14:39
@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label May 14, 2026
@k8s-ci-robot k8s-ci-robot requested a review from tallclair May 14, 2026 14:39
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 14, 2026
@vitorfloriano
Copy link
Copy Markdown
Contributor Author

/cc @Priyankasaggu11929

Copy link
Copy Markdown
Member

@Priyankasaggu11929 Priyankasaggu11929 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR, @vitorfloriano.

Just left one inline comment, but otherwise it looks good to me. Thanks!

always_run: true
skip_if_only_changed: "^docs/|\\.md$|^(README|LICENSE|OWNERS|SECURITY_CONTACTS)$"
branches:
- ^main$
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also run the job for release-v* branches

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

command:
- make
args:
- test
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question: make test generates a cover.out artifact. Should I do something with it? Or is it picked-up somehow by the TestGrid?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setting "decorate: true" inserts an env variable to the prow job container - $ARTIFACTS (usually pointing to /logs/artifacts/ path in the container)

so for cover.out or any other artifacts to show up in the Prow spyglass (prow.k8s.io) artifacts tab, copy/move these files to $ARTIFACTS directory path.

Prow sidecar component will pick up these files collected in $ARTIFACTS path and show them in Artifacts tab.

see for examples -
https://cs.k8s.io/?q=ARTIFACTS&i=nope&literal=nope&files=&excludeFiles=&repos=kubernetes/test-infra

and docs - https://docs.prow.k8s.io/docs/components/pod-utilities/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed. PTAL.

Copy link
Copy Markdown
Member

@Priyankasaggu11929 Priyankasaggu11929 May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vitorfloriano, apologies for suggesting it in parts.

Since we're going to move/run all our test suites for NRC repo in Prow now, and all of those tests will have artifacts of interest, so, how about we make the change in the Makefile itself - https://github.com/kubernetes-sigs/node-readiness-controller/blob/2239ccef203f6be9af3209ae7b8d146b9fccfb55/Makefile

at the top of the makefile, define an env variable ARTIFACTS defaulting to path /logs/artifacts

And then in the test make target - we can capture the cover.out at path something like $ARTIFACTS/cover.out?

Same we can do for other tests as well as we move.

That way, we don't have to do this copy/pasting of each artifact file to ARTIFACTS here in the job definition.

WDYT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Priyankasaggu11929 no problem. I thought about that at first, but then I considered that the make targets could be run locally and then I thought it would be better to move the artifacts here in the prowjob only.

But agreed, outputting to /logs/artifacts/cover.out seems more elegant. I'll open a PR in a minute.

@vitorfloriano vitorfloriano force-pushed the prow-node-readiness-controller-presub-test branch from ad959a5 to f44eeab Compare May 14, 2026 15:21
@vitorfloriano vitorfloriano marked this pull request as ready for review May 14, 2026 15:21
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 14, 2026
@k8s-ci-robot k8s-ci-robot requested a review from mrunalp May 14, 2026 15:21
@vitorfloriano vitorfloriano force-pushed the prow-node-readiness-controller-presub-test branch from f44eeab to 5d03968 Compare May 14, 2026 15:27
@vitorfloriano vitorfloriano force-pushed the prow-node-readiness-controller-presub-test branch from 5d03968 to dad8884 Compare May 14, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Issues or PRs related to code in /config area/jobs cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants